Round a fractional offset given as a string - #190

The inconsistency

date_zone_to_diff returns a Rational for a fractional-hour zone with more
than two digits (ext/date/date_parse.c:529-535), and the comment just above it
says that is intended — "no over precision for offset; 10**-7 hour = 0.36
milliseconds should be enough" (:506-508).

offset_to_sec's string branch threw that away: if (!FIXNUM_P(vs)) return 0;
at ext/date/date_core.c:2639.

Its two sibling branches in the same function do the opposite — they round and
warn. T_FLOAT at :2590-2592 and T_RATIONAL at :2621-2623 both emit
rb_warning("fraction of offset is ignored").

So one quantity, three ways, two answers:

DateTime.new(2001,2,3, 0,0,0, '+00.123')                #=> offset 0     "invalid offset is ignored"
DateTime.new(2001,2,3, 0,0,0, 442.8/86400)              #=> offset 443   "fraction of offset is ignored"
DateTime.new(2001,2,3, 0,0,0, Rational(2214, 5*86400))  #=> offset 443   "fraction of offset is ignored"

442.8 seconds either way. The string form — the only one a user actually types —
is the one that silently becomes +00:00.

Not currently tested

git ls-files -z test | xargs -0 grep -nE "['\"][-+][0-9]{2}\.[0-9]+['\"]" returns
nothing: no test passes a dotted zone as an offset argument. The .123 hits in
the suite are sub-second fractions of the time ('19990523235521.123456+0900'),
not zones, and grep -n 'fraction of offset' test/ finds nothing.

The one fractional zone that is tested, '[-9.50]' at test_date_parse.rb:137-138,
has two digits, so it takes date_zone_to_diff's n <= 2 Integer path
(date_parse.c:524-528) — already consistent, and untouched by this. The Rational
path is what was untested.

The change

Round it, exactly as the sibling branches do. k_rational_p, f_round,
f_eqeq_p and rb_warning are all already used in this file.

The k_rational_p guard matters: date_zone_to_diff returns Qnil for a zone it
rejects, and without the guard that nil reaches f_round.

Not a widening

'+24:00' and '+99:00' still give 0 — date_zone_to_diff rejects them before
this code runs. '+00:00' → 0, '+01.5' → 5400, '+9.50' → 34200, '+00.1' →
360, '+00.12' → 432, '+23:59:59' → 86399 are all unchanged. The
n < -DAY_IN_SECONDS || n > DAY_IN_SECONDS check at :2642 still runs on the
rounded value, and '+23.9999999' rounds to exactly 86400, which the inclusive
guard accepts — consistent with Rational(1,1) being legal per the existing
test_civil__offset at :196-197.

Direction

I want to be straight about the tension here. Read shallowly, the recent commits
on these files are a tightening trend (#183's range check, #188's commercial-week
validation, c98d85d verifying argument classes), which would argue for making the
string branch keep rejecting.

I do not think that is the right read. #183's own commit message frames its defect
as "the Integer 2 is rejected, but Rational(2,1) is the same quantity" — the
principle is consistency between representations of one quantity, which is exactly
this. And rounding is the long-standing behaviour here: rb_warning("fraction of offset is ignored") already exists at :2592, :2623 and :3349. The string
branch is the deviation, not the rule.

If you would rather go the other way and make Float and Rational reject too, that
is a coherent position and I am happy to write that instead.

Verification

• Full suite on CRuby 3.2.11: 148 tests, 162616 assertions, 0 failures, 0
  errors. Pristine control run: same 148 tests with exactly one failure, mine —
  so nothing else changes behaviour.
• TruffleRuby row verified, not inferred. That row was my one open question,
  since TruffleRuby ships its own date. It does compile this C extension via
  Sulong, and I ran it: with master's code '+00.123' gives 0 and warns "invalid
  offset is ignored"; with this patch it gives 443 and warns "fraction of offset is
  ignored", while '+24:00'/'+99:00' stay 0 and '+01.5'/'+23:59:59' are
  unchanged. So the truffleruby job is fixed by this too rather than broken by it.
• Mutation checks: truncating instead of rounding gives 442 and fails; dropping the
  warning fails; dropping the k_rational_p guard raises
  NoMethodError: undefined method 'round' for nil on the '+24:00' row.

One thing I deliberately left out

DateTime.parse('2001-02-03T00:00:00+00.123') goes through dt_new_by_frags,
which does of = NUM2INT(t) at date_core.c:8519 and truncates to 442. So
after this change DateTime.parse and DateTime.new differ by one second on the
same zone string.

That is a real but separate inconsistency in a different function, and bundling it
would make this patch harder to judge. Happy to follow up. (Same spot also narrows
before its range check at :8520, the inverse of #183's principle — currently
unreachable, since date_zone_to_diff caps output near 3.6e8.)

Disclosure

I used an AI assistant to help find and prepare this change. I reviewed and tested
it myself, and the outputs above are from runs I performed.